Remove/Address TODOs in onboarding #6116
Conversation
| }, | ||
| onNext = onOnboardingDone, | ||
| ) | ||
| // TODO: Consider making auth_code a value class to prevent string parameter mismatches |
There was a problem hiding this comment.
I tried it (I have a stash with the changes). I've used a value class in the AuthRepository to enforce a strong type but then this types is not properly use by the navigation library and it needs a custom NavType mapping which I don't like.
I hope this is going to work in nav3 let see, for now I'm going to keep it like this.
| if (isError) { | ||
| Text( | ||
| text = stringResource(commonR.string.manual_server_wrong_url), | ||
| // TODO probably wrong style and color/token |
There was a problem hiding this comment.
All TODOs regarding the color and sytle have been removed since the design team won't have time for us and for now it looks ok. We are going to need a more general change over the app later at least for the typographie.
TimoPtr
force-pushed
the
feature/onboarding-address-todos
branch
from
1a70c20 to
c286eb1
Compare
TimoPtr
changed the base branch from
feature/use_new_onboarding
to
feature/onboarding-rework-onboardapp
|
Last TODOs to address are to verify the URL for the documentation and it is going to be addressed in another PR. |
|
Taking "No TODO should be left from the onboarding code base" very seriously: Missed: What about:
|
Summary
This PR removes TODOs that we don't need anymore and address some of them when it makes sense.
Checklist
Any other notes
No TODO should be left from the onboarding code base the last one is going to be address in #6115
The split of the tests are in a dedicated commit that can be ignored.